fix: resolve framework version from an isolated framework venv - #1225

CI E2E report — ❌ Timeout

details

You're right, and thanks — I traced the whole chain and it holds exactly as you described.

build_manifest computes _prov = build_provenance(args, env=os.environ) at line 419, and
build_provenance does return stack_fingerprint (provenance.py line 355). The manifest then
threw that key away and called the local _detect_stack_fingerprint(), which resolves versions
by importing the package — an import that cannot succeed under --framework-env isolated. So
the fixed value was being computed and discarded on exactly the path that matters.

The downstream consequence is the one you named: cli/kb.py line 363 copies only non-unknown
values into stack_fingerprint_meta, and phases/explore.py lines 962-969 fills the specialist
framework_version from that same map, also skipping unknown. The report line was fixed
because it comes from platform_probe.py line 243, which calls the shared detector directly —
so the first commit fixed the one consumer that did not go through the manifest.

Worth adding: common/provenance.py's own module docstring already flagged this duplication
("manifest.py still keeps its own _detect_stack_fingerprint ... so those detectors are
currently duplicated here"), so the shared implementation was always the intended single source.

What the follow-up commit does

Takes the first of your two options. build_manifest now writes
_prov.get("stack_fingerprint") or {}, and the local detector is gone along with the
_read_first_line helper and the _STACK_FINGERPRINT_ENVS table that only it used — 144 lines
deleted against 31 added. The docstring above is updated so it no longer describes a duplication
that has been removed.

.get() rather than a subscript is deliberate:
test_build_manifest_shared_provenance_fields monkeypatches build_provenance with a stub that
omits the key, so a subscript would KeyError there.

One behaviour change to flag

The two detectors were not equivalent beyond the venv fallback. The manifest's version read
aiter.__commit__ (a git sha) first; the shared one goes through importlib.metadata and
returns a distribution version. So when aiter happens to be installed in the orchestrator's own
interpreter, that field records a version instead of a sha. AITER_COMMIT still takes priority
and is how the sha is normally supplied, and on the isolated bare-metal layout aiter is not in
that interpreter at all (the field is unknown today either way) — but it is a real change and
unavoidable once the two are unified. Say the word if you'd rather preserve the sha and I'll
carry the commit lookup into the shared detector.

Tests

Added test_manifest_versions_a_framework_installed_in_its_own_venv, which is the case you
asked for: only VLLM_VENV_ROOT set, no VLLM_VERSION, no vllm in the running interpreter, and
the asserted value is the written manifest's, not the detector's return. I checked it is not
vacuous — the interpreter it runs under raises PackageNotFoundError for vllm, so the assertion
can only pass through the venv fallback.

Removed the five tests that covered the deleted helpers, and the six
monkeypatch.setattr(mf, "_detect_stack_fingerprint", ...) seams, which would now raise
AttributeError on a name that no longer exists.

Green locally: 65 in the three directly-touched files, plus 360 across the manifest/session/KB
consumers (test_manifest_unit, test_manifest_units, test_framework_switch_manifest,
test_session_layout, test_cli_kb_unit, test_coordinator_kb_writes,
test_local_kb_requirements, test_warm_replay, test_preflight_serving_framework,
test_build_lifecycle). ruff check clean on all three files. I also confirmed no reference to
any of the removed symbols survives anywhere in the repo.

Following up on the aiter question I raised above — I dug into it and I'm keeping the current
behaviour. Reasoning, and one correction to what I said earlier.

Correction first: I wrote that AITER_COMMIT "is how that sha is normally supplied". That
overstated it. Nothing in the repo sets AITER_COMMIT — it appears only in the env-var priority
table and in tests, and no install/setup path writes it. So it is an available override, not a
mechanism anyone is currently using.

Why keep the shared implementation anyway:

1. This unifies an inconsistency rather than introducing one. platform_probe.py line 243 has
   been calling the shared detector all along, so the run report has always shown aiter as a
   distribution version. The sha only ever existed on the manifest side — the two consumers
   already disagreed, and this is what makes them agree.

2. Preserving the sha means import aiter, which is precisely what the shared detector avoids
   and says why: the import is heavy (seconds, may touch the GPU/driver, can trigger JIT module
   loads) and this code runs on the session-start path. Paying that on every run to sharpen one
   field in one deployment shape is the wrong trade, and it would contradict the design the
   module documents.

3. The affected surface is narrower than it sounds. On the isolated bare-metal layout aiter is
   not in the orchestrator's interpreter at all, so the field reads unknown before and after.
   Only a shared-environment install where aiter is importable changes, and it changes from a
   sha to a coarser version rather than losing the field.

The follow-up I'd suggest, separately: have setup record AITER_COMMIT in .env, exactly
the way it already records VLLM_VENV_ROOT — same pattern, same reason ("what the orchestrator
cannot see, setup writes down"). That restores sha-level precision for every layout without
putting an import on the session-start path, and it belongs in the setup path rather than in
this fix. Happy to open an issue for it, or to fold it in here if you'd rather it not wait.